-
Notifications
You must be signed in to change notification settings - Fork 792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ot_certs] add creator pubkey ID (serial number) to UDS certificate #21314
[ot_certs] add creator pubkey ID (serial number) to UDS certificate #21314
Conversation
A question about this setup: why does the attestation key ID need to come from something as heavy-duty as KMAC-KDF? The X.509 RFC suggests simply hashing the public key and using the hash as the ID. (It suggests SHA-1, but it also says any method that generates a unique ID is fine; I'd suggest SHA-256 given that's probably fastest on OT.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am no expert on the crypto part but overall it looks good.
uint8_t creator_pub_key_id[kCertKeyIdSizeInBytes] = {0}; | ||
|
||
// Generate the UDS key ID. | ||
uint32_t creator_pub_key_id[kOtCertPubkeyIdSizeIn32BitWords]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a fan of the uint32_t *
to uint8_t *
casting. I wonder if this should be done in ot_cert_gen_key_id
to have a well-defined and self-contained implementation of this hash since this is endian-specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cryptolib API mostly operates on 32-bit words. To not violate strict aliasing we can only alias with a a char
type, i.e., unsigned char *
. The cryptolib API requires the caller allocate space for the return value. Aliasing outside of ot_cert_gen_key_id
enables all these things to hold. I am not sure I follow your concerns regarding how this would impact endianness? Could you elaborate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I was just pointing out that the meaning of the cast from uint32_t *
to unsigned char*
depends on the endianness and is therefore a bit implicit for something which is quite important but as long as we have some tests to make sure that this produces the correct result, this is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has come up a few times with cryptolib. The crux of the issue is that while bytes are less ambiguous, words are safer from SCA attacks and faster, since dealing with more bits at a time obscures side-channel signal and the hardware registers mostly use words.
Like Tim said above, the current status is that the cryptolib mostly uses word buffers, but it always arranges the word buffers so that casting them directly to byte buffers with unsigned char *
on Ibex always produces the right byte stream.
Admittedly, KMAC was chosen here simply because it is what was specified in the OT Certificates specification document. It seems the rationale there was this block was hardened against SCA/Fault attacks (unlike the HMAC block). I think another choice for KMAC was the output size. The serial number field is 160 bits (convenient for a SHA1 or a cSHAKE KMAC). Upon further reflection, I am not sure this matters much for the serial number / subject key id certificate fields, as these can easily be verified out of band, since they are based on the public key, during certificate endorsement. I could just truncate the HMAC to 160bits? Wdyt @jadephilipoom @moidx ? |
c3ae679
to
38b3187
Compare
My understanding was that the key ID is something that anyone can derive from the public key anyway (provided they know the formula) so not sure the hardening brings anything. Is the time difference between HMAC and KMAC significant? After all, the attestation is not done very frequently. |
Yes, agreed @pamaury, I think the same applies for truncating an HMAC, just that collision resistance is reduced. But that should not matter much here I don't think. |
The language in the X.509 RFC specifically prefers hash functions[0], so I'd be in favor of using SHA-256 or SHA3-256 and truncating to 160 bits, rather than either HMAC or KMAC. My understanding is that the key identifier doesn't really need to be cryptographically secure, just deterministically based on the key so it can be used to distinguish certificates. [0] I'm looking at this passage here from RFC 5280:
|
The parameter names for the cert builder function matched the parameter names for the TbsCert builder function which was slightly misleading. Signed-off-by: Tim Trippel <[email protected]>
This updates the UDS cert generation code to add the creator pubkey ID, which is generated via a truncated SHA256 operation over the public key itself. The creator pubkey ID becomes the serial number for the UDS certificate. Signed-off-by: Tim Trippel <[email protected]>
38b3187
to
490e8de
Compare
Sounds sensible to me. I updated the PR to reflect the suggested changes. LMK what you think @pamaury @jadephilipoom . |
This has been merged but I am generally in favor of following the specification closely if possible so using HMAC sounds like a good idea to me. |
Post-merge LGTM! (Looks like GitHub doesn't let you explicitly approve after merge anymore.) |
This adds a utility function to compute attestation key IDs that are used in the "serial number" and "subject key ID" fields of attestation certificates. The attestation key ID is derived from an attestation public key using a KMAC KDF operation as described in NIST SP 800-56C.
Additionally, this updates the UDS certificate generation flow in the personalization firmware to include the creator pubkey ID field in the TBD certificate generation.